Skip to content

validate mesh#173

Merged
rajeeja merged 47 commits into
mainfrom
rajeeja/issue-141
Nov 13, 2023
Merged

validate mesh#173
rajeeja merged 47 commits into
mainfrom
rajeeja/issue-141

Conversation

@rajeeja

@rajeeja rajeeja commented Nov 8, 2022

Copy link
Copy Markdown
Contributor

@rajeeja rajeeja requested a review from erogluorhan November 8, 2022 17:52
@erogluorhan erogluorhan linked an issue Nov 9, 2022 that may be closed by this pull request

@erogluorhan erogluorhan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The flow suggested in this PR (i.e. calling validate() through Grid object) does not make sense to me because we are already creating any Grid, representing in the UGRID conventions, regardless of the original grid type coming from the file. In other words, any dataset that does not conform to UGRID standards would lead our code to throw exceptions in the Grid creation stage, wouldn't it?

I'd assume a flow instead like: ux.validate(xr.open_dataset(file)) (This would require us to modify the draft UXarray API, too)

Am I missing anything here?

@rajeeja

rajeeja commented Nov 14, 2022

Copy link
Copy Markdown
Contributor Author

The flow suggested in this PR (i.e. calling validate() through Grid object) does not make sense to me because we are already creating any Grid, representing in the UGRID conventions, regardless of the original grid type coming from the file. In other words, any dataset that does not conform to UGRID standards would lead our code to throw exceptions in the Grid creation stage, wouldn't it?

I'd assume a flow instead like: ux.validate(xr.open_dataset(file)) (This would require us to modify the draft UXarray API, too)

Am I missing anything here?

This is more for when the file is corrupt or the vertices or connectivity matrix that initialize the mesh are invalid and somehow passes the reader phase.

Eg. https://github.com/UXARRAY/uxarray/pull/173/files#diff-5c959492b91b5fdfc565d14eb2f672cf27b2f9722860c7efdba6a2109eb618a6R202

@rajeeja rajeeja self-assigned this Jun 7, 2023
@rajeeja rajeeja requested a review from philipc2 November 3, 2023 15:39
Comment thread uxarray/grid/grid.py Outdated
Comment thread uxarray/grid/grid.py Outdated
@rajeeja rajeeja requested a review from philipc2 November 13, 2023 16:44

@philipc2 philipc2 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! I'll approve after these two final comments are addressed.

Comment thread uxarray/grid/validation.py
Comment thread uxarray/grid/validation.py Outdated
@rajeeja

rajeeja commented Nov 13, 2023

Copy link
Copy Markdown
Contributor Author

Test failures seem to come from reliance on this ftp mesh file unavailability:
https://web.lcrc.anl.gov/public/e3sm/inputdata/share/meshes/mpas/ocean/oQU480.230422.nc
am I missing something @philipc2 ?

@philipc2

Copy link
Copy Markdown
Member

Test failures seem to come from reliance on this ftp mesh file unavailability: https://web.lcrc.anl.gov/public/e3sm/inputdata/share/meshes/mpas/ocean/oQU480.230422.nc am I missing something @philipc2 ?

This is correct. See #582

@rajeeja rajeeja requested a review from philipc2 November 13, 2023 20:19

@philipc2 philipc2 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work!

@rajeeja rajeeja merged commit d2f92a7 into main Nov 13, 2023
Comment thread docs/user_api/index.rst
Grid.to_polycollection
Grid.to_linecollection
Grid.to_shapely_polygons
Grid.validate_mesh

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One last catch, this should be

Suggested change
Grid.validate_mesh
Grid.validate

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

grid validate function

4 participants